Codecov Report

Attention: Patch coverage is 0% with 72 lines in your changes missing coverage. Please review.

Project coverage is 48.09%. Comparing base (7478f55) to head (c445d71).
Report is 127 commits behind head on main.

@@             Coverage Diff              @@
##               main     #269      +/-   ##
============================================
- Coverage     51.45%   48.09%   -3.37%     
- Complexity      406      434      +28     
============================================
  Files            30       31       +1     
  Lines          1028     1100      +72     
============================================
  Hits            529      529              
- Misses          499      571      +72     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Don't understand create duplicate PR of #257

@kuzmany guessing because there has been no response to feedback since September 2021!

Ok. But still would be good mention what is different between my version and this version.
We use that PR on production, works as expected and still don't see reason create new PR

Sure thing.

• Requires baseUrl to be given
• Implements accessTokenUpdated (for those who want it, gets set to true in requestAccessToken if nothing failed)
• Implements getAccessTokenData method to return token, expires and token type
• Checks the access token's expiry and presence in isAuthorized/validateAccessToken so code can re-use access tokens more easily
• Implements getQueryParameters, straight copy from Oauth class. I imagine it's there to fix a bug, based on the comment above it.
• Supports $this->_debug variable the same way Oauth class does

It would be great to get a decision made on this @escopecz @kuzmany and merge it if we want to accept it in favour of the outdated PR based on the feedback provided above.

@nick-vanpraet could you take a look at the failing tests for this PR please? Appreciate it was a while back, it'd be great to get it tested and merged!

@escopecz and @kuzmany we do still need to decide to merge this in favour of the older PR. It seems sensible to do so given the outlined additional functionality. Let's make a decision and get it merged?

@RCheesley we use #257 for our customers. It's from 2021 then I am not able to say what community need to merge. I see some Dennis comments, but cannot spend more time on it, only for maintenance to push it to branch. It's up to community. As usual I prefer less work for done.

Tested PR: #269 in my build server and it is working.

Just 2x NOTICES need to be fixed:

[Notice] Message: strlen(): Passing null to parameter #1 ($string) of type string is deprecated File: /vendor/mautic/api-library/lib/Auth/TwoLeggedOAuth2.php Line:158
[Notice] Message: strlen(): Passing null to parameter #1 ($string) of type string is deprecated File: /vendor/mautic/api-library/lib/Auth/TwoLeggedOAuth2.php Line:165

File: mautic\api-library\lib\Auth\TwoLeggedOAuth2.php

• at lines 158 and 165, change to use !empty( ... ) instead of strlen( ... )

public function validateAccessToken()
{
    $this->log('validateAccessToken()');

    //Check to see if token in session has expired
    if ( !empty($this->_access_token) && !empty($this->_expires) && $this->_expires < (time() + 10)) {
        $this->log('access token expired');

        return false;
    }

    //Check for existing access token
    if ( !empty($this->_access_token) ) {
        $this->log('has valid access token');

        return true;
    }

    //If there is no existing access token, it can't be valid
    return false;
}

This is a low risk merge:

1. Introduces a new file to extend available functionality.
2. Does not change or effect existing functionality or behavior.
3. Adheres to abstract and follows code standards and exception handling.
4. Improves README.MD with new documentation.

I vote to merge and released PR: #269 immediately, as it is a critical missing feature with low-impact.

@nick-vanpraet looks like there's some code style fixes to be done here, would you take a look please?

We also need to have the test coverage increased as you can see from CodeCov. Let us know if you need some help with that!